Skip to content

test(effect): Add Effect v3 compatibility tests for backwards compat#20389

Closed
JPeer264 wants to merge 2 commits intojp/effect-v4from
jp/effect-v4-compatibility-tests
Closed

test(effect): Add Effect v3 compatibility tests for backwards compat#20389
JPeer264 wants to merge 2 commits intojp/effect-v4from
jp/effect-v4-compatibility-tests

Conversation

@JPeer264
Copy link
Copy Markdown
Member

This adds Effect v3 compatibility tests as own folder. We can't really leave them inside packages/effect, as there we need to have Effect v4 as devDependency, so the old version can't be used nor tested anymore.

Also the new effect-3-compatibility-tests are not included in the yarn workspace. Here yarn somehow can't have two different effect versions installed and the tests just wouldn't work correctly (so I did an own yarn.lock inside this folder to keep them separate. In case we would move over to pnpm or something else we could try to add it as part of the workspace)

I basically copied over everything inside packages/effect/test into this folder and changed the createMetricsFlusher() to yield* TestClock.adjust('10 seconds');, as this is more reliable and wouldn't rely on any internal testing functionality.

@JPeer264 JPeer264 self-assigned this Apr 18, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 18, 2026

size-limit report 📦

Path Size % Change Change
@sentry/browser 25.78 kB added added
@sentry/browser - with treeshaking flags 24.27 kB added added
@sentry/browser (incl. Tracing) 43.65 kB added added
@sentry/browser (incl. Tracing + Span Streaming) 45.36 kB added added
@sentry/browser (incl. Tracing, Profiling) 48.58 kB added added
@sentry/browser (incl. Tracing, Replay) 82.79 kB added added
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 72.29 kB added added
@sentry/browser (incl. Tracing, Replay with Canvas) 87.49 kB added added
@sentry/browser (incl. Tracing, Replay, Feedback) 99.74 kB added added
@sentry/browser (incl. Feedback) 42.59 kB added added
@sentry/browser (incl. sendFeedback) 30.45 kB added added
@sentry/browser (incl. FeedbackAsync) 35.45 kB added added
@sentry/browser (incl. Metrics) 27.07 kB added added
@sentry/browser (incl. Logs) 27.2 kB added added
@sentry/browser (incl. Metrics & Logs) 27.89 kB added added
@sentry/react 27.53 kB added added
@sentry/react (incl. Tracing) 45.92 kB added added
@sentry/vue 30.61 kB added added
@sentry/vue (incl. Tracing) 45.49 kB added added
@sentry/svelte 25.8 kB added added
CDN Bundle 28.46 kB added added
CDN Bundle (incl. Tracing) 44.73 kB added added
CDN Bundle (incl. Logs, Metrics) 29.83 kB added added
CDN Bundle (incl. Tracing, Logs, Metrics) 45.81 kB added added
CDN Bundle (incl. Replay, Logs, Metrics) 68.73 kB added added
CDN Bundle (incl. Tracing, Replay) 81.68 kB added added
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 82.77 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback) 87.2 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 88.27 kB added added
CDN Bundle - uncompressed 83.12 kB added added
CDN Bundle (incl. Tracing) - uncompressed 133.75 kB added added
CDN Bundle (incl. Logs, Metrics) - uncompressed 87.27 kB added added
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 137.17 kB added added
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 210.63 kB added added
CDN Bundle (incl. Tracing, Replay) - uncompressed 250.99 kB added added
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 254.38 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 263.9 kB added added
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 267.29 kB added added
@sentry/nextjs (client) 48.44 kB added added
@sentry/sveltekit (client) 44.09 kB added added
@sentry/node-core 57.94 kB added added
@sentry/node 174.78 kB added added
@sentry/node - without tracing 97.89 kB added added
@sentry/aws-serverless 115.12 kB added added

JPeer264 added a commit that referenced this pull request Apr 21, 2026
This adds support to Effect v4, but also keeps the compatibility for v3.
There is no way that we can unit test against v3, as the
`devDependencies` need to use `effect@4` and an updated `@effect/vitest`
version, which is not compatible with Effect v3 (this is added in
#20389).

The API for Effect v4 has changed a little, so there are safeguards to
detect if it is v3 or v4 and uses the correct API. The good part is that
for users nothing changed, so they still can use the same methods in
their app as before (ofc, respecting the new Effect v4 API).

Before (Effect v3):

```ts
const SentryLive = Layer.mergeAll(
  Sentry.effectLayer({
    dsn: '__DSN__',
    tracesSampleRate: 1.0,
    enableLogs: true,
  }),
  Layer.setTracer(Sentry.SentryEffectTracer),
  Logger.replace(Logger.defaultLogger, Sentry.SentryEffectLogger),
  Sentry.SentryEffectMetricsLayer,
);
``` 

After (Effect v4):

```js
const SentryLive = Layer.mergeAll(
  Sentry.effectLayer({
    dsn: '__DSN__',
    tracesSampleRate: 1.0,
    enableLogs: true,
  }),
  Layer.succeed(Tracer.Tracer, Sentry.SentryEffectTracer),
  Logger.layer([Sentry.SentryEffectLogger]),
  Sentry.SentryEffectMetricsLayer,
);
```

Both usages still work and are represented in the E2E tests.
@JPeer264 JPeer264 marked this pull request as ready for review April 21, 2026 07:44
@JPeer264 JPeer264 requested review from nicohrubec and s1gr1d April 21, 2026 07:45
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit c328112. Configure here.

'@sentry-internal/browser-integration-tests') }}
changed_effect_v3_compatibility:
${{ needs.job_get_metadata.outputs.changed_ci == 'true' || contains(steps.checkForAffected.outputs.affected,
'@sentry-internal/effect-3-compatibility-tests') }}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI affected detection will never trigger for this package

Medium Severity

The changed_effect_v3_compatibility output uses contains(steps.checkForAffected.outputs.affected, '@sentry-internal/effect-3-compatibility-tests'), but this package is intentionally excluded from the yarn workspace (confirmed it's absent from the workspaces list in root package.json). Since NX only tracks workspace projects, @sentry-internal/effect-3-compatibility-tests will never appear in the affected list. On PRs, the job condition needs.job_build.outputs.changed_effect_v3_compatibility == 'true' will only be true when changed_ci is true (CI config changes), never when the test files themselves or @sentry/effect source code changes. This means the compatibility tests effectively won't run on most PRs.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit c328112. Configure here.

Copy link
Copy Markdown
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests look fine, but I am not sure if we should have this as a top-level folder in the dev-packages. Maybe you can put it in an existing folder or create a new one version-compatibility-tests (or something more fitting for general library/framework tests)

Copy link
Copy Markdown
Member

@nicohrubec nicohrubec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other packages afaik the pattern if we support multiple versions is to keep one set of unit tests and test version compatibility via multiple e2e tests. I am wondering if this would also be sufficient here. Is there a particular reason you think this should be treated differently?

@JPeer264
Copy link
Copy Markdown
Member Author

Tests look fine, but I am not sure if we should have this as a top-level folder in the dev-packages

I was also not very sure where to put it

For other packages afaik the pattern if we support multiple versions is to keep one set of unit tests and test version compatibility via multiple e2e tests

We still have E2E tests, I wasn't sure if that is sufficient, but on a second thought E2E tests might do it as well - as I wanted the API to be typesafe and testable with the old @effect/vitest. But in theory the type safety can be guaranteed with E2E tests and once this works it should work with @effect/vitest as well.

I'll close this one for now and will revisit once there are any specific issues with not having unit tests.

@JPeer264 JPeer264 closed this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants